refactor: 응답에서 refresh token은 제외하여 전달하도록 #646
Conversation
whqtker
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
sukangpunch
left a comment
sukangpunch
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 궁금한 점 리뷰 달았습니다.
| boolean isRegistered, | ||
| String accessToken, | ||
| String refreshToken) implements OAuthResponse { | ||
| @JsonIgnore String refreshToken) implements OAuthResponse { |
There was a problem hiding this comment.
제 생각에 Response 객체는 http 응답 본문을 나타내는 객체라고 생각하여 명확하게 엑세스 토큰과 isRegisterd 필드만 담은 response dto를 따로 두어 컨트롤러에서 매핑하여 리턴하는 방식은 어떠신가요?
There was a problem hiding this comment.
저도 동의합니다~ isRegisterd이건 왜 필요한건가요??
전 내부에서 쿠키에 세팅하는 객체를 예를들어
public record SignInResult(
AccessToken accessToken,
RefreshToken refreshToken
) { }
이런식으로 쓰고
controller에서 응답용으론
public record SignInResponse(
String accessToken
) { }
이렇게 하면 되는 거 아닌가 했는데 놓친 히스토리가 있나 싶어서요!
There was a problem hiding this comment.
그러네요 코드에서는 isRegistered 필드를 참조하는 로직이 없네요 이 부분도 말씀하신 내용 반영하면서 수정하겠습니다 !
| boolean isRegistered, | ||
| String accessToken, | ||
| String refreshToken) implements OAuthResponse { | ||
| @JsonIgnore String refreshToken) implements OAuthResponse { |
There was a problem hiding this comment.
저도 동의합니다~ isRegisterd이건 왜 필요한건가요??
전 내부에서 쿠키에 세팅하는 객체를 예를들어
public record SignInResult(
AccessToken accessToken,
RefreshToken refreshToken
) { }
이런식으로 쓰고
controller에서 응답용으론
public record SignInResponse(
String accessToken
) { }
이렇게 하면 되는 거 아닌가 했는데 놓친 히스토리가 있나 싶어서요!
3 participants
관련 이슈
작업 내용
로그인 응답에서 refresh token은 제외하도록
@JsonIgnore어노테이션을 붙였습니다.아예 해당 필드는 제거할 수는 없는게, 응답 DTO에서 refresh token 필드를 통해 쿠키를 설정하고 있습니다. 따라서 프론트에게 전달만 되지 않도록 설정했습니다.
더 좋은 방법이 있다면 말씀해주세요 ~
추가로
sign-up엔드포인트에는 쿠키로 리프레시 토큰을 전달하고 있지 않아서(응답으로 전달하고 있었음), 쿠키로 전달하도록 추가 반영했습니다 !특이 사항
리뷰 요구사항 (선택)